fix(core-internal): make zod toJSONSchema conversion wire-truthful for tool schemas - #2556
fix(core-internal): make zod toJSONSchema conversion wire-truthful for tool schemas#2556claude[bot] wants to merge 37 commits into
Conversation
…r tool schemas
The SDK validates tool payloads with the user's zod schema but ships the
raw object, so the advertised JSON Schema must describe that raw shape.
Pass zod-scoped conversion options (via libraryOptions on the Standard
JSON Schema path, and directly on the zod 4.0-4.1 fallback):
- unrepresentable: 'any' so one z.date()/z.bigint() field no longer
throws and fails the entire tools/list response
- rewrite z.date() to {type: 'string', format: 'date-time'}, the shape
JSON.stringify actually produces for a Date
- for output schemas, drop .default()-carrying fields from required and
drop additionalProperties: false on plain z.object() (kept for
z.strictObject()), so validating clients accept legitimate
structuredContent the server ships as returned
Fixes #2464
Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 53f2be5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
There was a problem hiding this comment.
Beyond the inline findings, the input-side analogue of the date rewrite was also examined and ruled out: advertising z.date() inputs as string/date-time while server-side zod validation rejects strings is not a regression — pre-PR such a tool failed the entire tools/list, so no previously-working flow breaks, and the behavior matches the v1 converter (#2467); z.coerce.date() remains the way to accept string dates.
Extended reasoning...
Two real bugs plus a nit were found and posted inline, so approval is off the table and the inline comments already convey the need for human review. This note only records the one additional candidate that was examined and refuted this run — the input-schema z.date() advertise/validate mismatch — so a later pass doesn't re-explore it from scratch. It is informational, not a correctness guarantee.
- Keep user annotations (.describe()/.meta()) on rewritten z.date()
fields: under unrepresentable: 'any' the node carries only annotation
keywords, so the previous wipe loop deleted exactly the user's
metadata and nothing else.
- Key the output required-filter on the zod shape instead of the
emitted default keyword: a registered .default() hides its default
behind a $ref (and stayed required), a .meta({default}) annotation on
a genuinely-required field was wrongly dropped, and undefined-
accepting fields (z.any(), z.unknown(), z.undefined()) stayed
required even though JSON.stringify drops undefined-valued keys from
the wire. A field is now dropped from required iff validating
undefined succeeds (missing key semantics).
- Opt the elicitation path out of the graceful-degradation options via
a new unrepresentable: 'throw' conversion option: a z.date()
rewritten to string/date-time would pass the wire checks, but the
accepted response could never satisfy z.date() on handler re-entry,
so acceptedContent() would silently discard the user's answer.
z.date() keeps throwing the documented TypeError before anything is
sent.
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Beyond the inline findings, I also examined whether advertising z.date() input fields as string/date-time creates a new mismatch with server-side validation — tools/call does validate arguments with the raw zod schema (mcp.ts validateToolInput), so a string date fails that individual call with InvalidParams. Ruled out as a new issue: pre-PR the same schema failed the entire tools/list (every tool disappeared), so a per-call validation error is strictly less severe, and it matches the v1/zod-3 converter's long-standing behavior for z.date() inputs.
Extended reasoning...
Bugs were found this run (unhandled-rejection crash risk in the fieldAcceptsMissingKey probe, and the required-filter gap for enum-keyed records), so approval is not appropriate; the inline comments carry those findings. This note only records the one additional candidate examined and refuted: the input-side z.date() advertised-vs-validated mismatch is pre-existing v1 behavior and an improvement over the pre-PR total tools/list failure, not a new defect introduced by this PR.
… records
- fieldAcceptsMissingKey no longer lets a throwing .transform()/.refine()
escape the probe: depending on the zod build, ~standard.validate(undefined)
either throws synchronously or returns a rejecting Promise whose unhandled
rejection crashes the process during tools/list conversion. Both modes now
conservatively keep the field required, and the Promise branch attaches a
no-op catch so no rejection floats.
- Enum-keyed records (def.type === 'record') also emit a required list zod
does not enforce when the shared value schema is defaulted or
undefined-accepting; the override now drops it (all-or-nothing, since every
key shares the one value schema).
- Move StandardSchemaToJsonSchemaOptions above the conversion function's
JSDoc block so the doc comment re-attaches to the function it describes,
and give the interface its own one-liner.
- Stop overclaiming in the changeset and JSDoc: BigInt values embedded as
defaults or metadata (.default(0n), .meta({default: 1n})) still fail
conversion inside zod's own processors, outside the override's reach.
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Beyond the inline nit, this round also examined and ruled out two input-side candidates: (1) input (io: 'input') schemas keeping required on enum-keyed records / undefined-accepting fields — not the #2464 failure class, since an over-listed input required only makes clients supply keys validation would tolerate omitting; no legitimate payload is rejected. (2) the z.date() → string/date-time rewrite on input schemas creating an unsatisfiable per-call contract — the resulting tools/call validation failure is a loud InvalidParams error (matching v1 converter behavior), strictly better than the pre-PR whole-tools/list failure, and the elicitation path now opts out via unrepresentable: 'throw'.
Extended reasoning...
Bugs were found this run (one nit on pipe/transform/coerce output schemas, posted inline), so this note only records what else was examined and refuted. The prior two rounds' findings (date annotation wipe, JSON-keyed required proxy, record required, unhandled probe rejection, elicitation date path, JSDoc attachment) are all addressed in commits 034ccc1 and d11d6fc — verified against the current source. The PR remains non-trivial core conversion logic and should still get a human look before merge; no approval.
…oerce outputs Output schemas containing .transform()/.pipe()/z.coerce still advertise the post-transform shape (io: 'output') while the server validates and ships the raw pre-transform value — a pre-existing mismatch this PR does not address. Note it as a known residual gap in the zodConversionOptions contract and scope the changeset claim accordingly, instead of rewriting pipe nodes in the override: a per-node input-side re-conversion would break $refs to registered schemas (a nested conversion's $defs land at the wrong document root), and advertising output schemas with input semantics wholesale is a design decision that interacts with SEP-2106 non-object output roots. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Beyond the inline finding, this run also examined the symmetric input-side concern — that rewriting z.date() (and degrading z.bigint()) in inputSchema advertises argument shapes the tool's zod validation can never accept (a JSON string never satisfies z.date() on tools/call) — and ruled it out as a defect of this PR: those schemas were entirely unlistable before (the whole tools/list threw), so no working case regresses, and the rewrite matches the v1/zod-3 converter's long-standing behavior. Recording it here so a later pass doesn't re-explore it.
Extended reasoning...
Bugs were found this run (one nit inline: the zod 4.0–4.2.x isParent guard can skip the override on clone-parent nodes, making the fix inert on the declared peer floor while the lockfile's zod 4.3.6 hides it from tests). Per the review process, the inline comment already signals the needed attention; this comment only records that the input-side z.date()/z.bigint() advertisement concern was independently examined and refuted (no regression vs. pre-PR total tools/list failure, and v1 converter parity), so future review passes need not re-derive it.
…chemas
On zod 4.0-4.2.x, toJSONSchema guards the override hook with
'if (!seen.isParent)' (v4/core/to-json-schema.js), and a schema instance
is marked isParent whenever a clone of it (.describe()/.meta()) appears
in the same conversion — so a schema reused both bare and via a clone
skips sanitization on the bare node: untruthful required/
additionalProperties survive and z.date() emits {}. zod 4.3.0 removed
the guard, so the lockfile resolution (4.3.6) and the test suite cannot
observe it. Verified against the published 4.2.1 and 4.3.0 tarballs.
Document it as a known residual gap and scope the changeset claim;
bumping the declared zod floor (^4.2.0 -> ^4.3.0) would close it for
the primary path but is a maintainer decision.
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Beyond the inline findings, this pass also examined the input-side rewrite — z.date() in an inputSchema advertised as string/date-time while tools/call validates arguments with the raw zod schema — and ruled it out: it matches the deliberate v1 #2467 / zod-v3 converter behavior this PR cites, and it strictly improves on the pre-PR failure mode (the entire tools/list dying), so it is a design precedent rather than a bug introduced here.
Extended reasoning...
This run's three findings are all nit-level residual gaps (async-staged defaults / .catch() nodes missing from the documented gap list, non-object unrepresentable roots bypassing the input-root guard, and the z.date() divergence over the non-serializing InMemoryTransport) — none is a regression, and each follows the prose-scoping pattern the author already applied in prior rounds. Separately, a finder raised the input-side date/unrepresentable rewrite as advertising argument shapes the server's own tools/call validation rejects; verification confirmed validateToolInput does run the raw zod schema (mcp.ts), but the rewrite is the same intentional choice made in the v1 #2467 fix and the zod v3 converter, and pre-PR the identical registration failed the whole listing, so it was ruled out as established design rather than a defect of this PR. Recording it here so a later pass does not re-explore it.
…e-root gaps
- fieldAcceptsMissingKey short-circuits on def.type 'default'/'prefault'
before the validate(undefined) probe: a defaulted field accepts a
missing key by construction, but an async stage (.refine(async ...))
pushed the probe to a Promise and conservatively kept the field in the
advertised required list while async-capable server validation accepts
the omission.
- Output .catch() nodes degrade to an unconstrained schema (annotations
and the emitted default kept): catch-validation accepts any raw value
— the fallback replaces it only in the parsed result, which the
server never ships — so advertising the inner constraints made
validating clients reject legitimate results.
- The input-root guard keys on the emitted type, which
unrepresentable: 'any' erases for bare z.bigint()/z.map()/z.set()/
z.symbol() roots — they were stamped {type: 'object'} and advertised
as permanently-uncallable tools. Recover the signal from the zod def
so misregistered roots keep throwing the actionable 'must describe
objects' error.
- Document the InMemoryTransport transport-dependence of the z.date()
string/date-time advertisement (pass-by-reference, no JSON
round-trip) as a known residual gap in the JSDoc and changeset.
Co-Authored-By: Claude <noreply@anthropic.com>
…er root guard
- Skip the .catch() degrade at the conversion root: deleting
'type: object' there flipped the 2025-era codec's legacy-wrap
predicate (isNonObjectJsonSchemaRoot), silently shipping
structuredContent as {result: ...} to 2025-era peers for root-level
.catch() output schemas that were object-rooted pre-PR.
- Preserve x-* vendor-extension keys through the nested .catch()
degrade, mirroring the elicitation walker's annotation-only
convention — they carry no validation constraint.
- Extend NON_OBJECT_UNREPRESENTABLE_ROOTS with 'void', 'undefined',
'nan', and 'function': all degrade to a typeless {} under
unrepresentable: 'any' and can never accept a JSON object, so they
must keep throwing the actionable root error (z.custom() stays
excluded — it can legitimately accept objects).
- Document the input-side z.date() round-trip impossibility (advertised
string/date-time vs raw-zod input validation) as a known residual
gap in the JSDoc and changeset, pointing at z.iso.date()/
z.iso.datetime() as the supported input spellings.
Co-Authored-By: Claude <noreply@anthropic.com>
… the root guard
- Extend the .catch() degrade bail from the conversion root to every
position that feeds the output epilogue's root object proof (members
of root-level, possibly nested, anyOf/oneOf/allOf compositions):
degrading such a member broke isProvablyObjectShapedRoot's
every-member proof, left the root typeless, and flipped the 2025-era
legacy wrap — silently shipping structuredContent as {result: ...}
for previously-working union/intersection output schemas.
- The input-root guard now unwraps the zod def chain (optional/
nullable/readonly/default/prefault/catch via innerType, lazy via its
getter with a seen-set cycle guard) before consulting the non-object
set, so z.bigint().optional() and z.lazy(() => z.bigint()) no longer
become phantom {type: 'object'} tools; 'literal' joins the set
(typeless literal roots — unrepresentable values like
z.literal(undefined) or mixed-type value lists — cannot describe
objects; representable single-type literal roots already throw via
the explicit-type guard).
- Document dynamic catch values (.catch(ctx => ...)) as a known
residual gap: zod's catchProcessor throws before the override hook
runs, so the degrade covers static fallback values only; changeset
claim scoped to match.
Co-Authored-By: Claude <noreply@anthropic.com>
…romise roots - The catch-degrade verdict must be position-independent: zod deduplicates reused schema instances and runs the override once per instance, so a .catch() shared between a nested position and a root(-composition) position got one verdict applied to both sites — nested-first broke the root object proof (2025-era legacy-wrap flip), root-first kept unenforced inner constraints at the nested copy (the #2464 client-rejection class). The degrade now always keeps the emitted 'type' (dropping properties/required/additionalProperties and other constraints), which preserves the root proof at every position; the position-dependent feedsRootObjectProof branch is removed and the stale JSDoc bullet reworded to the new rule. - unwrappedZodDefType now unwraps pipe nodes via their INPUT side (def.in — the side io: 'input' conversion and input validation consume; never def.out) and promise nodes via innerType, so z.bigint().transform(...), z.bigint().pipe(...), and z.promise(z.bigint()) roots throw the actionable 'must describe objects' error instead of becoming phantom {type: 'object'} tools. z.object({...}).transform(...) stays accepted, and bare standalone z.transform(fn) stays excluded like z.custom(). Co-Authored-By: Claude <noreply@anthropic.com>
…uired, catch-of-union skeleton
- The input-root guard now recurses into compositions via a new
nonObjectTypelessRootType helper: a union root is rejected when EVERY
member unwinds to a non-object typeless type (one representable
object member keeps it accepted), an intersection when ANY side does
(the value must satisfy both) — so z.union([z.bigint(), z.symbol()])
and z.intersection(z.bigint(), z.bigint()) throw the actionable root
error again instead of becoming phantom {type: 'object'} tools.
'nonoptional' joins the transparent-wrapper set (safe:
z.object({...}).nonoptional() emits an explicit type: 'object').
- fieldAcceptsMissingKey also returns true for fields whose unwrapped
def type is 'symbol' or 'function': JSON.stringify drops Symbol- and
function-valued keys from the payload by the same mechanism that
drops undefined-valued keys, so no serialized result can carry them
(covers both the object required-filter and the record branch).
- The .catch() degrade reduces composition keywords (anyOf/oneOf/allOf,
emitted when the catch wraps a union or intersection) to member type
skeletons instead of deleting them: the catch node emits no 'type'
key for these shapes, so the keep-type rule alone left the root
typeless and flipped the 2025-era legacy wrap for previously-working
registrations; the JSDoc bullet is updated to the full rule.
Audited sibling shapes: z.file() roots already throw via the explicit
type guard (representable as string/binary); z.never() and
fully/mixed-representable non-object unions were silently stamped
pre-PR too (status quo, not regressed here); catch-of-$ref roots were
typeless pre-PR as well (no wrap change) and now correctly drop the
unenforced $ref constraints; BigInt-valued output fields make
JSON.stringify throw (a transport-level ship failure already noted for
defaults in the changeset).
Co-Authored-By: Claude <noreply@anthropic.com>
…-only catch type
- Fixes a regression introduced by the composition recursion: 'literal'
in the reject set classified fully-representable members like
z.literal('admin') as non-object, so
z.union([z.literal('admin'), z.literal('member')]) — zod's idiomatic
enum spelling, which converted and listed fine pre-PR — threw and
failed the ENTIRE tools/list (and prompts/list) with -32603, the exact
whole-list outage this change eliminates. A composition member now
counts as non-object ONLY when it unwinds to a genuinely
unrepresentable type (bigint/symbol/map/set/void/undefined/nan/
function) or to a literal whose values are unrepresentable or
mixed-type; representable members (single-type literals, z.string(),
z.null()) keep the composition accepted exactly as pre-PR.
- Fixes the fail-open direction of the same recursion: the shared
seen-set conflated cycle protection with the member verdict, so
const b = z.bigint(); z.union([b, b]) returned a false may-be-object
verdict. The guard now tracks only the current traversal path
(ancestors), giving shared instances a real verdict at every
occurrence while recursive lazies stay bounded.
- The .catch() degrade keeps 'type' only when it is 'object': a
non-object type is an unenforced constraint that rejects the
wrong-typed raw values catch exists to tolerate, and the 2025-era
legacy-wrap object proof only ever consumes type === 'object' (same
rule in compositionTypeSkeleton). z.number().catch(0) now advertises
{default: 0}.
- Generalize the input-side residual-gap prose beyond z.date(): any
REQUIRED unrepresentable input field (z.bigint()/z.map()/z.set()/
z.symbol()) makes the tool listed yet uncallable via JSON — use
JSON-representable types or make the field optional (JSDoc +
changeset).
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Beyond the inline findings, this pass also examined whether mixed-type literal roots/unions (e.g. z.literal(['a', 1]) at root position) wrongly regress into a whole-tools/list throw — ruled out: such values are provably non-object (never callable via JSON), the root guard's loud rejection is the intended behavior for misregistered roots, and the PR's own tests pin both this rejection and the acceptance of representable literal unions (z.union([z.literal('admin'), z.literal('member')])).
Extended reasoning...
Bugs were found this run and are posted as inline comments, so no approval. This note records the one additional candidate examined and refuted this run — mixed-type literal roots failing the whole tools/list — so later review passes don't re-explore it from scratch. The refutation rests on the guard's every-member rule (representable members keep compositions accepted, pinned in this PR's tests) and on the loud-failure convention for provably non-object roots that earlier review rounds converged on. This is informational only, not a correctness guarantee.
…wind piped defaults
- Composition member classification: 'date' joins
NON_OBJECT_UNREPRESENTABLE_TYPES — a Date value can never be a JSON
object, and at member position the rewritten string/date-time node
nests inside anyOf/allOf where the explicit-type root guard cannot
see it, so z.union([z.date(), z.date()]),
z.union([z.date(), z.bigint()]), and
z.intersection(z.object({...}), z.date()) were stamped as phantom
{type: 'object'} tools (pre-PR each threw loudly). Bare date ROOTS
are unaffected (they throw via the explicit-type guard after the
rewrite). isNonObjectTypelessLiteral now also rejects non-finite
number literal values (Infinity/-Infinity/NaN are typeof 'number'
but cannot ride JSON). Mixed unions with a representable member
(z.union([z.date(), z.string()])) stay accepted under the
every-member rule.
- fieldAcceptsMissingKey's structural default short-circuit read only
the outermost def type, missing a default hidden inside a pipe:
z.number().default(7).transform(async v => v) is outer-'pipe' with
the ZodDefault at def.in, so the probe went async and the field
wrongly stayed in the advertised output required list while
async-capable validation fills the default. A new
hasStructuralDefault helper unwinds pipe INPUT sides, lazies, and
transparent wrappers looking for a default/prefault node (verified:
every wrapper in the set fills the inner default on undefined).
Co-Authored-By: Claude <noreply@anthropic.com>
…table literals; broaden tolerance detection
- oneOf skeletons: catch-of-discriminated-union output schemas (zod
emits DUs as oneOf) kept the oneOf keyword while members reduced to
identical {type: 'object'} skeletons, so every legitimate payload
matched BOTH members and Ajv rejected 'must match exactly one schema
in oneOf' — a strict regression vs pre-PR where the constrained
members were disjoint. The degrade loop and compositionTypeSkeleton
now emit oneOf skeletons under anyOf, the honest loosening once the
discriminating constraints are stripped (wrap-neutral:
isProvablyObjectShapedRoot treats the composition keywords
identically).
- isNonObjectTypelessLiteral: drop the jsonTypes.size > 1 branch — a
mixed-but-REPRESENTABLE literal (z.literal(['a', 1]) emits a valid
{enum: ['a', 1]}) listed silently pre-PR, and classifying it
non-object made one such registration fail the entire tools/list, a
round-10-introduced fail-closed regression. Only genuinely
unrepresentable values (undefined/bigint/symbol/non-finite numbers)
reject now, matching the loudness-parity contract.
- hasStructuralDefault -> hasStructuralMissingKeyTolerance: recognize
static .catch() BEFORE the wrapper unwind would step past it (any
input, including undefined, is replaced by the fallback — tolerance
by construction), and recurse into union def.options with ANY-member
semantics (a union accepts a missing key whenever any member does;
intersections deliberately excluded). Fixes catch/union-nested
defaults with async stages staying in the advertised output required.
Co-Authored-By: Claude <noreply@anthropic.com>
… loud-literal guard; keep anchors - The round-21 some()-semantics for allOf in isProvablyObjectShapedRoot fired on UNTOUCHED intersection emissions: z.intersection(z.object, z.bigint()) output stamped-and-listed before the loudness guard ran (pre-#2464 it threw), and z.intersection(z.object, z.any()) — a working registration — got stamped, silently flipping the 2025-era wire shape. Every composition key now uses EVERY-member semantics again (main's decision for untouched emissions); the all-keys iteration — the actual round-21 repair — stays, and the loosen rewrite's relocated {anyOf: members} conjunct is itself provably object-shaped, so its proof survives. Additionally the output epilogue consults the loud verdict BEFORE the stamp, so a loud conjunct throws even when an object conjunct could prove the root. - isBigintValuedLiteralRoot -> isLoudLiteralOutputRoot: adds the io-appropriate pipe branch (recurse def.out with the bare-transform fallback to def.in, mirroring the verdict walk) and replaces the bigint-only typeof check with nonObjectLiteralLoudness(values) === 'loud' — catching codec spellings (z.number().transform(() => 1n) .pipe(z.literal(1n)) emits {type: 'number', const: 1}) and z.literal([undefined, 'a']) (zod filters the undefined value and emits an explicit type), both of which threw pre-#2464. - The catch degrade no longer deletes $anchor/$dynamicAnchor/$id: reference TARGETS constrain no instance value, and deleting them dangles inbound $refs — the advertisement failed Ajv COMPILE and every callTool failed pre-send. They stay on the node (and remain movable by wrapConstraintsInAnyOf). Co-Authored-By: Claude <noreply@anthropic.com>
…intersections; allOf-push type stamp
- The reference-target keep was incomplete at two sibling sites:
compositionTypeSkeleton now carries member $anchor/$dynamicAnchor/
$id into skeletons (a catch-of-union member's anchor with a sibling
$ref alias dangled), and $defs joins the catch delete-loop skip (a
pure reference container — deleting it dangled cross-subtree $refs).
Both loosen-neutral; both previously made the advertisement fail Ajv
COMPILE, so every callTool failed pre-send.
- The quiet output-path date verdict flowed into INTERSECTION
positions: z.intersection(z.object({...}), z.date()) output listed as
a permanently-broken tool (no value satisfies both sides; pre-#2464
it threw, and the input path still throws). The intersection branch
now upgrades a quiet date side to loud on the output path —
parity-safe, since every date-containing intersection threw
pre-#2464; bare/nullable/union-with-representable date outputs keep
listing (pinned).
- allOf-push residual: a .meta() carrying BOTH a non-all-object anyOf
AND a non-object-provable allOf conjunct (e.g. {minProperties: 1})
defeated the every()-member proof on every composition key after the
push, flipping the 2025-era wrap for a working registration. The push
branch now stamps the sound explicit type: 'object' itself when every
relocated member is object-provable (the value must satisfy the
pushed conjunct), preserving the pre-#2464 stamp without touching the
proof semantics — the round-22 untouched-intersection fixes stay
intact. The stale '(allOf uses some-semantics)' test title is renamed
to match.
Co-Authored-By: Claude <noreply@anthropic.com>
…d oneOf; date-union parity; $defs keeps
- Restore first-present-key-wins in isProvablyObjectShapedRoot: untouched
multi-key roots (e.g. a nullable union with a user .meta({allOf})) keep
their typeless root and 2025-era legacy wrap; the loosen rewrite's
allOf-push stamps its own type and no longer relies on the epilogue proof.
- Redirect same-document $ref/$dynamicRef JSON Pointers through segments the
oneOf->anyOf rewrite relocates (rename: /oneOf/<i> -> /anyOf/<i>; push:
/oneOf/<i> -> /allOf/<n>/anyOf/<i>), mirroring the legacy wrap's
position-aware, $id-scoped pointer rewrite - a dangling alias pointer made
the document uncompilable.
- Treat an all-date union as a date side at output intersections: no value
satisfies both conjuncts, so parity keeps them loud (direct-side and root
verdicts unchanged).
- Keep $defs path-addressable: the catch degrade's type skeleton carries it
like the name-resolved anchors, and wrapConstraintsInAnyOf leaves it at the
wrap site instead of relocating it into anyOf[0].
Co-Authored-By: Claude <noreply@anthropic.com>
…tralization; mixed date-union parity - Defer the catch degrade's (and the skeleton's) oneOf->anyOf rename to the epilogue's rewriteOneOfToAnyOf: the override hook has no document path, so renames performed there could not record the pointer move that keeps inbound $ref aliases resolvable. Members skeletonize under their emitted key; the epilogue - which always runs once the degrade set loosened - renames them at a known path with move bookkeeping. - Record the array/tuple null-tolerance wraps' relocations (items -> items/anyOf/0, prefixItems/<i> equivalents) via the override's ctx.path so the existing redirect pass covers them; on zod versions without ctx.path the wrap still ships and dangling pointers fall through to neutralization. - Neutralize same-document pointers left unresolvable by the degrade's constraint deletes (no move target exists for a deleted subtree): loosen-only removal of the reference keyword, scoped to loosened conversions, run after the redirect pass so relocated-but-surviving targets stay referenced. - Extend the union date-side rule to mixed quiet members: a union whose every member is a date or a quiet never/file/non-finite-literal verdict admits at most Dates, so output intersections with it stay loud (a date-free union of nevers keeps listing). Co-Authored-By: Claude <noreply@anthropic.com>
…re neutralization; structural date-intersection parity
- Extend the dangling-ref neutralization to the two remaining locally-verifiable
forms: anchor-form '#name' refs whose $anchor/$dynamicAnchor rode a subtree
the degrade deleted (anchors are relocation-immune but dangle by deletion -
surviving names are collected document-wide, a keep-erring superset of the
resource-scoped lookup), and base-addressed '<uri>#/...' refs into embedded
$id resources the loosen rewrite mutated (verified against the embedded
resource and dropped; document-rooted moves cannot redirect them). Truly
cross-document refs stay untouched.
- Make neutralization polarity-aware: a dangling ref under not/if/contains no
longer has its $ref deleted (not: {} rejects everything; if: {} force-fires
then; a looser contains tightens against maxContains) - the enclosing boundary
keyword is removed at its outermost occurrence instead, which only ever
loosens since top-level keywords AND-combine. The redirect pass stays
polarity-insensitive by design.
- Replace the thrice-patched date-intersection enumeration with the structural
rule: an atIntersection flag threads through the verdict recursion so a date
leaf reports loud at any nesting depth under an intersection side, and
representable non-object types (string/number/boolean/null/enum/
template_literal, representable literals) return DEFINED quiet
'representable' verdicts instead of undefined - reserving the may-be-object
bail for genuinely satisfiable members (z.union([z.date(), z.object({...})])
intersections keep listing) while a representable member still discharges
union loudness outside intersections (z.union([z.date(), z.string()]) stays a
working schema on both io paths).
Co-Authored-By: Claude <noreply@anthropic.com>
…ref repair; neutralization fixpoint + annotation siblings; probe-based wrap tolerance - Add 'array' and 'tuple' to REPRESENTABLE_NON_OBJECT_ZOD_DEF_TYPES: a JSON array is never a JSON object, so array/tuple union members must return the defined quiet verdict instead of falling to the may-be-object bail that laundered loud date/bigint co-members at intersections (the bigint spelling on both io paths). Plain object-with-array intersections stay quiet. - Treat embedded $id resources as sub-documents in the neutralize pass: the loosen machinery mutates freely inside them while the document-rooted repair walks skipped them, leaving base-relative refs inside a resource dangling after a rename/wrap/degrade. The walk now recurses with the resource node as resolution root (renames inside a resource are neutralized rather than redirected - moves are document-rooted), and the boundary scan checks inside resources with per-resource predicates. - Iterate neutralization to a fixpoint: a boundary-conjunct deletion is itself a fresh dangle source (deleted $anchor judged against a stale snapshot; pointer-form refs into a conjunct deleted later in the walk), so each round re-derives targets until nothing is deleted. When deleting a conjunct, also drop its annotation-consuming siblings (contains -> unevaluatedItems; if -> then/else + unevaluated*), closing the 2020-12 annotation channel where the bare deletion was a tightening. - Use fieldAcceptsMissingKey (structural walk + validate(undefined) probe) at the array/tuple wrap sites, matching the object required-filter and record branches, so probe-only-tolerant elements (z.preprocess fallbacks) get the null wrap too. Co-Authored-By: Claude <noreply@anthropic.com>
…e-construct guard; may-be-object verdicts
Reverse the repair-after-mutation approach for reference keywords: instead of
redirecting/neutralizing every reference form the loosen family can break
(pointer redirects, anchor collection, embedded-$id sub-documents, RFC 3986
base resolution, polarity-aware fixpoint neutralization), DETECT hand-authored
reference constructs up front and skip the loosen family for those conversions.
- The zod output conversion now runs a strict pass first (date rewrite +
draft-04 id strip only) and inspects the natural emission: any
$ref/$dynamicRef beyond zod's own registry shapes ('#', '#/$defs/<name>'),
any $anchor/$dynamicAnchor, any $id, or a non-root $defs ships that
strict pre-#2464-shaped emission - compilable and working by construction,
at the cost of the loosening (documented as a Known residual gap).
Reference-free documents (and zod's own registry/recursion refs, stable
under every loosen mutation - $defs stays at the root, entry names are
untouched, entries mutate in place) convert again with the loosen family
active, exactly as before.
- Delete the entire repair machinery: move recording, pointer redirect pass,
dangling-ref neutralization with fixpoint iteration, polarity boundaries,
annotation-sibling cleanup, per-resource recursion, anchor/resource
collection. This closes the residual classes reported against it (per-ref
deletion vs unevaluated* annotations, embedded-$id resources at array
positions, verbatim-string $id base matching and boundary-deleted
resources) by construction.
- Fix the may-be-object union bail laundering loud members past provably
non-object intersection conjuncts: a union with an object/any/custom member
now returns a distinguishable quiet 'mayBeObject' verdict carrying the
members' inner loudness, which the intersection branch surfaces only when
the sibling side is provably non-object (object-conjunct intersections keep
listing; array/string-conjunct spellings with a date or bigint member throw
as pre-#2464, on both io paths for bigint).
Co-Authored-By: Claude <noreply@anthropic.com>
|
Design shift in 21ae868: the reference-repair machinery is replaced by a conservative reference-construct guard. Four review rounds surfaced successive residuals in the machinery that repaired references after the wire-truthfulness loosening mutated the emitted document (pointer redirects, anchor collection, embedded- This commit reverses the approach:
The tradeoff is documented as a Known residual gap (JSDoc + changeset): schemas carrying hand-authored reference keywords keep pre-fix strictness — Generated by Claude Code |
…larity and unevaluated* clauses in the reference guard - The allOf-push branch stamps its enforced type: 'object' only on a genuine instance-typing proof (every relocated member carries an explicit type: 'object', recursively through nested compositions - EVERY oneOf/anyOf branch, ANY allOf conjunct) instead of isProvablyObjectShapedRoot's keyword-presence rule: properties/required are vacuous for non-object instances, so a hand-authored type-less member satisfiable by 42 must not be stamped away. Zod-emitted DU members and catch skeletons always carry the explicit type, so the 2025-era legacy-wrap protection is unaffected. - The reference-construct guard flags two more hand-authored spellings that observe in-place loosening as a tightening: any $ref/$dynamicRef - registry-shaped included - consumed under a not/if/contains polarity boundary (the lexical polarity skip cannot see through the ref indirection, so a negated consumer of a loosened $defs entry inverts the loosening), and any unevaluatedProperties/unevaluatedItems (annotation consumers that lose contributions when a catch degrade or required-filter strips their contributors; zod never emits these keywords). Positive-polarity registry-shaped aliases keep loosening, pinned. Co-Authored-By: Claude <noreply@anthropic.com>
…d vocabulary; root-aware allOf-push stamp; prose sync
- The tuple null-tolerance wrap reassigns a FRESH prefixItems array on the
emitted node instead of writing elements in place: a .meta({prefixItems})
array is the user's registry-owned object (zod's Object.assign meta merge
shares it by reference and the override runs before the terminal deep
clone), so the in-place write permanently corrupted registry metadata,
nested one more anyOf per tools/list conversion, and leaked false
null-tolerance into input advertisements.
- The guard's vocabulary now covers the legacy-draft spellings the SDK's
Ajv2020 (strict:false) engine still enforces: draft-07 'dependencies' joins
both keyword sets (walked as a name->schema map like dependentSchemas; the
array-of-strings form walks harmlessly), and any
$recursiveRef/$recursiveAnchor occurrence is flagged as a hand-authored
reference construct (zod never emits them; a negated {$recursiveRef: '#'}
observes root loosening as a tightening exactly like its $dynamicRef
successor).
- The allOf-push stamp is root-aware: at the conversion ROOT it stamps on the
keyword-presence heuristic (byte-parity with main's epilogue stamp, whose
first-present-key-wins read the emitted oneOf - declining flipped the
2025-era legacy wrap whenever an unrelated .default() fired the loosen
pass), while nested nodes keep the genuine explicit-type proof.
- Sync the changeset paragraph and the Known-residual-gaps bullet with the
full set of loosening-disabling conditions (polarity-consumed refs,
unevaluated*, $recursiveRef/$recursiveAnchor).
Co-Authored-By: Claude <noreply@anthropic.com>
…additionalProperties drop sets loosened; additionalItems in guard vocabulary - The 2025-era legacy-wrap stamp decision now reads the STRICT pre-loosen emission the guard pass already computes (snapshot isProvablyObjectShapedRoot(strict); the output epilogue consults it, falling back to the post-loosen proof only when no strict pass ran - guard-shipped results and non-zod vendors). Main decided the wrap on the raw emission, so this is byte-parity by construction: loosen mutations that destroy proof-relevant keys (the allOf-push relocating a root oneOf, the catch degrade deleting meta-authored required/properties) or expose keys main's first-present-key-wins rule never read can no longer flip the SEP-2106 wire shape on an unrelated trigger, in either direction. The allOf-push branch's stamping arm (both the root keyword-presence and nested explicit-type proofs) is deleted - the snapshot covers the root, and nested nodes never received any stamp pre-#2464. - The plain-object additionalProperties: false drop now sets loosened.value: it was the only loosen mutation that skipped the flag, so a solo drop left a hand-authored exactly-one oneOf over registry-hoisted plain objects reject-everything (both $defs entries become mutually satisfiable without the rename). Plain-object DU emissions now rename to anyOf - harmless, discriminator consts keep members exclusive - pinned, with a strictObject control keeping its oneOf. - Add draft-07 'additionalItems' to SCHEMA_CARRYING_JSON_SCHEMA_KEYWORDS: the SDK's cfworker provider enforces it in every draft mode and collects refs inside it, so a hand-authored ref there must trip the guard - parallel to the round-30 'dependencies' fix. Co-Authored-By: Claude <noreply@anthropic.com>
…the guard path; snapshot honors main's decision order; prose sync - hasStructuralMissingKeyTolerance no longer unwinds through 'nonoptional' as transparent: z.nonoptional() RE-FORBIDS undefined, so acceptance-tolerance inside it (.optional(), z.any(), ...) does not survive the wrapper while filling-tolerance (default/prefault/static catch) does - the structural walk claims nothing and the validate(undefined) probe in fieldAcceptsMissingKey decides, keeping .required() fields truthfully advertised as required and no longer spuriously setting loosened. - The zod output flow defers the draft-04 id strip on its strict pass: when the reference guard fires, the shipped emission keeps the id keys a URI-form hand-authored ref resolves through on the cfworker engine (schema.$id || schema.id base registration) - stripping made a working pre-#2464 registration permanently uncallable. Fragment-only guard documents still get a post-hoc strip so Ajv keeps compiling registry-id documents; Ajv rejected URI-form-ref documents pre-#2464 too, so keeping id there is pre-fix parity. (Bare draft-04 id occurrences deliberately do NOT trip the guard: every registered schema emits one, and the dangerous combination - an id-base URI-form ref - already trips it via the ref shape.) - strictRootProven honors main's decision order: an explicit non-object type on the strict root short-circuits before the object proof, so meta-authored object keywords on a scalar catch root can no longer flip the SEP-2106 wrap after the degrade deletes the type. - Changeset: document the oneOf->anyOf rename (near-universal for plain-object DU emissions now that the additionalProperties drop sets the flag) and the serialized-wire-form wraps (array/tuple null, file {}, non-finite null); rewrite two stale inline comments still describing the deleted pointer-move bookkeeping. Co-Authored-By: Claude <noreply@anthropic.com>
…rm ref-aware id strip; prose sync - The pipe branch of hasStructuralMissingKeyTolerance claims IN-side tolerance only when the OUT side is a bare transform (nothing re-validates the filled value - the pinned async-transform shape): a validating OUT side may reject it (.default(0).pipe(z.number().min(1)); .optional().pipe(z.coerce.number()) coerces undefined to NaN), so the walk claims nothing there and the validate(undefined) probe decides - .required()-style truthful required advertisements, no spurious loosened flag. The preprocess direction keeps its structural claim (deferring to the probe would mis-require async-refined preprocess fields) - the undefined-unsafe-fn residual is documented in the Known residual gaps list. - The draft-04 id strip is now uniformly ref-aware: the INPUT path defers the strip like the output paths and applies it post-hoc, and the gate is tightened from fragment-ness to the guard's registry-shape test - cfworker resolves even fragment pointers INSIDE an id resource relative to that base, so ANY hand-authored ref (URI-form or fragment-form) keeps the id (exact pre-fix parity; Ajv rejected those documents pre-fix too), while registry-only documents keep getting the strip for Ajv compilability. - Prose sync: retitle the stale allOf-push-stamps test to the strict-snapshot mechanism, fix the matching isProvablyObjectShapedRoot comment, and document the id strip in the changeset (what disappears, why, the cfworker caveat). Co-Authored-By: Claude <noreply@anthropic.com>
… symbol wire-drop through nonoptional; context-aware id-strip gates
- Three more false-tolerance spellings defer to the validate(undefined) probe:
.prefault(v) feeds v THROUGH the inner schema (filling-then-revalidating is
not filling - .min(1).prefault(0) rejects a missing key), intersections
claim structural tolerance only for the provably-mergeable distinct-key
plain-object-defaults shape (zod throws Unmergable intersection for two
scalar defaults with different fills; the pinned async-refined
object-defaults spelling keeps its structural claim), and z.promise gets a
claims-nothing branch in this walk (zod 4's promise parse rejects undefined
outright; the wrapper stays transparent for the root-type-verdict walks).
- The nonoptional branch recognizes SERIALIZATION-drop tolerance: a
symbol/function leaf can never appear on the wire regardless of validation,
so it survives the re-forbid (z.object({s: z.symbol().optional()})
.required() drops s like the bare spelling) while acceptance tolerance
still does not propagate. Added the Known-residual-gaps bullet for
async-staged validating pipe OUT sides (neither structural direction is
sound; conservative stay-required is pre-fix parity).
- The id-strip gates are context-aware: any ref lexically inside a draft-04
id-carrying resource counts as hand-authored (cfworker resolves refs there
base-relatively, and zod never emits a bare '#' inside an id entry - the
strip silently INVERTED validation verdicts on both io paths), and
$recursiveRef values count regardless of shape (zod never emits the
keyword). Registry-only documents keep getting the strip.
Co-Authored-By: Claude <noreply@anthropic.com>
| // `catch` and `optional` must be recognized BEFORE the wrapper unwind below | ||
| // would step past the very node granting tolerance (bare `.optional()` fields | ||
| // are already excluded from `required` by zod's emitter, but one inside a pipe | ||
| // — `z.string().optional().transform(async ...)` — is not). | ||
| if (def.type === 'default' || def.type === 'catch' || def.type === 'optional') return true; | ||
| if (def.type === 'prefault') { | ||
| // UNLIKE `.default()`, `.prefault(v)` feeds v THROUGH the inner schema — | ||
| // `z.number().min(1).prefault(0)` rejects a missing key. Filling-then- | ||
| // revalidating is not filling: claim nothing and let the probe decide | ||
| // (sync verdicts are correct both ways; an async-refined valid-prefault | ||
| // field conservatively stays required, matching the documented posture). | ||
| return false; | ||
| } | ||
| if (def.type === 'any' || def.type === 'unknown' || def.type === 'undefined' || def.type === 'void') return true; | ||
| if (def.type === 'symbol' || def.type === 'function') return true; | ||
| if (def.type === 'literal' && Array.isArray(def.values) && def.values.includes(undefined)) return true; |
There was a problem hiding this comment.
🔴 Two residual false-tolerance spellings in the structural missing-key walk drop genuinely-required fields from the advertised output required (zod's own toJSONSchema keeps them) and spuriously fire the document-wide oneOf→anyOf rename: (1) the tolerance-granting early returns for default/catch/any/unknown (standardSchema.ts:925/934) are checks-blind — z.number().default(0).refine(v => v >= 1) claims tolerance while safeParse of a missing key FAILS (the check on the default node re-validates the filled 0); (2) plainObjectDefaultFill (lines ~1040-1047) accepts ANY non-null/non-array object as 'plain', so two distinct Date/Map/class-instance default fills are vacuously key-disjoint and pass intersectionSidesFillDisjointObjects while zod's mergeValues throws 'Unmergable intersection' on every payload omitting the key. Fix both with the established pattern: claim tolerance only when def.checks is empty/absent, and require Object.getPrototypeOf(fill) === Object.prototype || null — everything else defers to the validate(undefined) probe, whose verdict is correct in every verified sync spelling.
Extended reasoning...
The two residual holes
Both are residual siblings of the fixes this PR already shipped for the same class — nonoptional (37d8eb4), pipe (74ab2bd), and prefault/intersection/promise (a0a0252) — with root causes none of those touch. In both cases the structural true short-circuits fieldAcceptsMissingKey before the validate(undefined) probe runs, so the probe's correct not-tolerant verdict is unreachable.
Hole 1 — checks-blind tolerance-granting early returns. zod 4 attaches .refine()/.check() to the same node (a clone with def.checks populated), but the early returns at packages/core-internal/src/util/standardSchema.ts:925 (default/catch/optional) and :934 (any/unknown/undefined/void) decide from def.type alone and never consult def.checks. The walk's own premise — "zod fills the default before any refinement runs" — is exactly backwards for checks attached to the default node itself: zod fills the default, then the check on that node re-validates and rejects the fill.
Hole 2 — non-plain default fills. plainObjectDefaultFill (lines ~1040-1047) gates only on typeof fill !== 'object' || fill === null || Array.isArray(fill). Dates, Maps, Sets, and class instances all pass, and — having zero own enumerable keys — are vacuously disjoint under intersectionSidesFillDisjointObjects's Object.keys test. But zod's mergeValues requires isPlainObject (prototype === Object.prototype or null) or +a === +b for Dates; anything else throws Unmergable intersection. The implementation is strictly weaker than the invariant the a0a0252 fix's own JSDoc states ('BOTH sides plain objects with disjoint key sets').
Step-by-step proofs (all verified empirically on HEAD, zod 4.3.6, by three independent verifiers per finding)
Checks-blind (1):
const s = z.object({ f: z.number().default(0).refine(v => v >= 1), name: z.string() })— verifieddef.type === 'default'withdef.checks.length === 1(refine attaches to the SAME node, not a pipe, so none of the prior fixes cover it).s.safeParse({ name: 'n' })FAILS: zod fills 0, the check on the default node rejects it — every legitimate payload must carryf.- zod's own
z.toJSONSchema(s, { io: 'output' }).requiredtruthfully keeps['f', 'name']; the SDK advertises['name']. - The
validate(undefined)probe returns issues (the correct verdict) but is unreachable behind the structuraltrue. - Same false drop for
z.number().catch(0).refine(v => v >= 1)andz.any()/z.unknown().refine(v => v !== undefined); the union-member spelling propagates; and the array call site advertisesitems: {anyOf: [..., {type: 'null'}]}while[1, null, 3]does NOT parse — a null wrap no legitimate payload can carry.
Non-plain fills (2):
const m = z.intersection(z.date().default(new Date(0)), z.date().default(new Date(86400000)))insidez.object({ m, name: z.string() }).safeParse({ name: 'n' })THROWS'Unmergable intersection. Error path: []'— the field is categorically required.- zod's own emission keeps
required: ['m', 'name']; the SDK advertises['name']. - Control: same-timestamp Date fills (
+a === +b) genuinely merge and the probe returns a sync tolerant verdict — so deferring non-plain fills to the probe is correct in both directions. Distinct Map fills and class-instance fills reproduce identically.
Secondary effect (both holes, verified): the false tolerance spuriously sets loosened.value, firing the document-wide oneOf→anyOf rename — a sibling strict-object discriminated union, the exact shape this PR's own test pins as keeping its exactly-one oneOf, had its oneOf renamed to anyOf merely because a checks-carrying default (or non-plain-fill intersection) coexisted in the same document.
Why existing safeguards miss it, and why the 'loosen-only' defense doesn't apply
fieldAcceptsMissingKey runs the probe only when the walk claims nothing; here the walk claims tolerance, suppressing the accurate verdict. The walk's 'all checks err loosen-only' JSDoc covers only client-side validation of shipped payloads — but the result is still a wire-truthfulness regression versus zod's own truthful emission (the PR's stated goal), the array-element null wrap advertises values that can never validate, and the spurious loosened flag rewrites unrelated strict compositions document-wide, contradicting the PR's own pinned strict-DU test. The same argument applied to the pipe and prefault siblings, both of which the author accepted and fixed as blocking. This also matches the repo REVIEW.md's Completeness recurring catch: a partial migration leaving sibling code paths with the very bug the prior commits fixed.
Fix
Same pattern as the three prior sibling fixes: (1) in the default/catch/any/unknown/void/undefined-literal early returns, claim tolerance structurally only when def.checks is empty/absent — tolerance is then certain by construction — otherwise return false and let the probe decide (verified: the probe is synchronous and correct in every sync spelling above); (2) in plainObjectDefaultFill, add const proto = Object.getPrototypeOf(fill); if (proto !== Object.prototype && proto !== null) return undefined; — mirroring zod's isPlainObject gate. The pinned plain-object-fills intersection test keeps passing.
Honest trade for (1): the pinned test "a defaulted field with an async stage is still dropped from output required" uses .default(0).refine(async () => true), whose default node carries a check — under the fix it defers to the probe, goes async, and conservatively stays required. That is exactly the posture the Known-residual-gaps JSDoc already adopts for async-staged validating pipe OUT sides (an async check's acceptance of the fill is equally unprovable), so the pinned expectation should flip to match — or the gap documented alongside that bullet.
| function hasHandAuthoredRefValues(document: Record<string, unknown>): boolean { | ||
| return someSchemaNode(document, (record, insideIdResource) => | ||
| ['$ref', '$dynamicRef', '$recursiveRef'].some(refKey => { | ||
| const value = record[refKey]; | ||
| if (value === undefined) return false; | ||
| if (refKey === '$recursiveRef') return true; | ||
| if (typeof value !== 'string' || insideIdResource) return true; | ||
| return value !== '#' && !ZOD_REGISTRY_REF_PATTERN.test(value); | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔴 The context-aware id gates treat every ref lexically inside a draft-04 id-carrying $defs entry as hand-authored, but zod's own emitter places registry-shaped #/$defs/<name> refs inside id entries for recursive (z.lazy self-ref under .meta({id})) and cross-registered schemas — so those schemas keep the Ajv-fatal id keyword on both io paths (every Client.callTool re-validation fails at compile with 'NOT SUPPORTED: keyword "id"', the exact failure the changeset claims fixed) and silently lose the entire #2464 loosen family on output. Fix: inside id resources, flag only bare # and non-registry shapes as hand-authored, and keep exempting #/$defs/<name> root-base pointers, which zod itself emits there and which resolve through the document root, not the id base.
Extended reasoning...
The bug
The a0a0252 context-aware gates decide hand-authored-ness of refs by lexical context: inside a draft-04 id-carrying resource, every ref counts as hand-authored — hasHandAuthoredRefValues (standardSchema.ts:556, if (typeof value !== 'string' || insideIdResource) return true) and hasHandAuthoredReferenceConstructs (~line 676, if (inIdResource) return true). The premise, stated in both JSDoc blocks, is that zod never emits ref shapes lexically inside an id-carrying entry. That premise is empirically false for the #/$defs/<name> shape: zod's own emitter places exactly those refs inside id entries for two common registry idioms:
- Recursive registered schemas:
const Category: z.ZodType<Cat> = z.object({name: z.string(), friend: z.lazy(() => Category).optional()}).meta({id: 'Category'})emits$defs.Category = {..., properties: {friend: {$ref: '#/$defs/Category'}}, id: 'Category'}— the self-ref sits lexically inside the id entry with zero hand-authoring anywhere. - Cross-registered schemas:
B = z.object({q: z.string()}).meta({id: 'BEntry'}); A = z.object({b: B, n: z.number().default(1)}).meta({id: 'AEntry'})—$defs.AEntrycarries{$ref: '#/$defs/BEntry'}inside its id resource.
Only the bare '#' spelling is never emitted there (the previous round's finding, which this gate correctly fixed — this bug is that fix's overshoot in the opposite direction).
Step-by-step proof (reproduced on HEAD a0a0252, zod 4.3.6, repo's own tsx)
const Category: z.ZodType<Cat> = z.object({ name: z.string(), friend: z.lazy(() => Category).optional() }).meta({ id: 'Category' });
const schema = z.object({ cat: Category, counted: z.number().default(0), name: z.string() });- The strict emission carries
$ref: '#/$defs/Category'inside the id-carrying$defs.Categoryentry, sohasHandAuthoredRefValuesreturns true → the deferredstripLegacyIdKeywordsnever runs; on the output pathhasHandAuthoredReferenceConstructsalso fires → the guard ships strict and the loosen pass never runs. - Result, both io paths:
$defs.Category.id === 'Category', andnew AjvJsonSchemaValidator().getValidator(result)throwsNOT SUPPORTED: keyword "id", use "$id"at COMPILE — every SDK-clientcallToolagainst such a tool fails client-side re-validation. This directly contradicts the shipped changeset: 'Registry metadata (.meta({id: 'X'})) no longer emits the draft-04idkeyword on either io path … The key is kept only when the document carries a hand-authored ref beyond zod's registry shapes' — no hand-authored ref exists here. - Result, output path: the entire zod v4: toJSONSchema options are hardcoded — z.date() breaks tools/list; output schemas advertise required/closed shapes the raw structuredContent can't satisfy #2464 loosen family is silently disabled — verified
required === ['cat', 'counted', 'name'](defaultedcountedstays required), and in the cross-registered case$defs.AEntrykeepsadditionalProperties: falsewithrequired: ['b', 'n']despiten = .default(1). The PR's headline fix (validstructuredContentomitting a defaulted field / carrying an extra key passes client validation) does not apply, contradicting the guard's own JSDoc ('All of these can only enter an emission through.meta()/registry metadata — i.e. they are hand-authored') and the changeset ('zod's own registry refs, stable under every mutation … loosen freely'). - Control (gate works as intended elsewhere): a non-recursive
z.object({q}).meta({id: 'RegPlain'})whose refs sit only outside the entry gets the strip (id === undefined, pinned by this PR's own tests) — the misfire is specifically refs-inside-id-entries, which zod itself emits.
Why the conservatism buys nothing
The keep-the-id posture is justified for genuinely hand-authored refs (they may resolve through the id base on cfworker). But zod's #/$defs/<name> pointers are ROOT-base JSON Pointers: with the id kept, cfworker resolves them base-relatively inside the id resource and they dangle (Unresolved $ref '#/$defs/Category' at validator build — verified by the verifiers on both pre- and post-PR code), so keeping the id preserves pre-PR brokenness on cfworker while newly-broken-by-omission on Ajv relative to the changeset's promise. With the id stripped, Ajv compiles AND cfworker compiles with the recursive ref genuinely enforcing. Stripping is strictly safe for these shapes.
The fix
Inside id resources, flag only bare '#' and non-registry shapes as hand-authored (the direction the prior round's fix actually needed), and keep exempting #/$defs/<name> root-base pointers — zod itself emits them there, and they resolve through the document root, not the id base. This preserves the previous fix (a hand-authored $ref: '#' inside an id resource still keeps the id) while letting zod's own emissions get the strip and the loosening. Apply the same carve-out to both hasHandAuthoredRefValues and the inIdResource clause in hasHandAuthoredReferenceConstructs.
Severity
Not a regression vs main (main also emitted the Ajv-fatal id and had no loosening), but all the gate code is new in this PR, and this is squarely the repo's Completeness recurring catch (the PR claims to fix registry-id Ajv compilability and wire-truthful loosening, and leaves the recursive/cross-registered registry idiom — zod's documented registry use case — with the very bugs the PR claims to fix) plus the Documentation & Changesets catch (the changeset makes a now-false claim about .meta({id}) on 'either io path'). Concrete failure if merged as-is: tools using recursive or cross-registered .meta({id}) schemas list but fail every SDK-client callTool re-validation at Ajv compile, while the changeset promises the opposite.
| function hasSerializationDroppedLeaf(field: unknown): boolean { | ||
| if (typeof field !== 'object' || field === null) return false; | ||
| const def = (field as { _zod?: { def?: { type?: string; innerType?: unknown } } })._zod?.def; | ||
| if (def === undefined || typeof def.type !== 'string') return false; | ||
| if (def.type === 'symbol' || def.type === 'function') return true; | ||
| if (WRAPPER_ZOD_DEF_TYPES.has(def.type) && def.innerType !== undefined) return hasSerializationDroppedLeaf(def.innerType); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🟡 The new hasSerializationDroppedLeaf helper (standardSchema.ts:1016-1023) is narrower than the main tolerance walk it shadows: it handles only direct symbol/function leaves plus transparent-wrapper unwinds, with no union-member recursion and no lazy-getter branch — so the symbol wire-drop tolerance the a0a0252 fix restored through .nonoptional()/.required() is still lost for union and lazy spellings (e.g. z.object({s: z.union([z.symbol(), z.string()]).nonoptional(), name: z.string()}) advertises required: ['s','name'] while a legitimate symbol-valued result ships as {"name":"n"} and fails the SDK's own client-side re-validation). Fix by mirroring the main walk: recurse union members with ANY-member semantics and follow lazy getters with the same try/catch + cycle bound.
Extended reasoning...
The bug
hasSerializationDroppedLeaf (packages/core-internal/src/util/standardSchema.ts:1016-1023) exists to carry SERIALIZATION-based tolerance through the nonoptional re-forbid — a symbol/function-typed value can never appear on the wire (JSON.stringify drops the key) no matter what validation demands, and the validate(undefined) probe cannot see that (the a0a0252 fix's own comment: the tolerance "survives the re-forbid" and "the probe cannot see" it). But the helper recognizes only two shapes: a direct symbol/function leaf, and WRAPPER_ZOD_DEF_TYPES unwinds. The main structural walk it shadows, hasStructuralMissingKeyTolerance, additionally recurses union members (ANY tolerant member suffices) and follows lazy getters with a try/catch and a cycle-bounding ancestors set. Those two spellings are missing here, so the nonoptional branch (return hasSerializationDroppedLeaf(def.innerType)) returns false for them, and the probe then correctly reports validation-required — which is the wrong question for symbol values, since validation-required and wire-present diverge exactly for this class.
The code path
For z.object({s: z.union([z.symbol(), z.string()]).nonoptional(), name: z.string()}) on the output path: the required-filter calls fieldAcceptsMissingKey(shape.s) → hasStructuralMissingKeyTolerance hits the nonoptional branch → hasSerializationDroppedLeaf(union) → def.type === 'union' matches neither the leaf check nor WRAPPER_ZOD_DEF_TYPES → false. The probe then runs validate(undefined), which the union (re-forbidden by nonoptional) rejects, so s stays in required. Same for z.lazy(() => z.symbol()).nonoptional() — the lazy getter is never invoked.
Step-by-step proof (verified empirically on HEAD a0a0252, zod 4.3.6, by four independent verifiers)
standardSchemaToJsonSchema(z.object({s: z.union([z.symbol(), z.string()]).nonoptional(), name: z.string()}), 'output').required→['s', 'name'].- Control: the bare spelling
z.object({s: z.union([z.symbol(), z.string()]), name: z.string()})correctly advertisesrequired: ['name'](pinned by this PR's own 'union-wrapped symbols' test) — the same serialization tolerance is honored bare but lost through the re-forbid, which changes nothing about serialization. schema.safeParse({s: Symbol('x'), name: 'n'}).success === true— the server legitimately validates and ships this result.JSON.stringify({s: Symbol('x'), name: 'n'})→{"name":"n"}— the keyscan never be on the wire when the matched member is the symbol.- A validating client (including the SDK's own
Client.callToolAjv re-validation) rejects the payload with "must have required property 's'". - The lazy spelling
z.object({s: z.lazy(() => z.symbol()).nonoptional(), name: z.string()})reproduces identically:required: ['s','name'], symbol value accepted by validation, key dropped by serialization.
Why existing code doesn't prevent it
The pinned test 'symbol leaves keep their wire-drop tolerance through nonoptional' covers only the direct .required()-on-optional-symbol spelling, which the helper's leaf + wrapper-unwind coverage handles. Neither the probe (validation-only) nor any other loosening path can rescue the union/lazy spellings.
Impact and fix
Impact is bounded: this is loosen-direction completeness only. Pre-PR emissions kept these fields required identically (the whole loosen family is new in this PR), so nothing that worked on main breaks — the affected registrations were equally broken before. The trigger is also exotic: a symbol- or function-typed OUTPUT field reached via union or lazy under .nonoptional()/.required(), with an actually-symbol-valued result. That said, it is the same residual-sibling class as the previously-accepted nonoptional finding (the earlier inline comment implemented in a0a0252), and per the repo's Completeness recurring catch the surviving spellings of a fixed pattern are worth closing.
Fix: mirror the main walk's coverage in hasSerializationDroppedLeaf — recurse union options with ANY-member semantics (whenever the symbol member is the matched one, the key vanishes from the wire), and follow lazy getters inside a try/catch with the same ancestors-set cycle bound the sibling walks use. Pipe sides could be added for completeness but have no realistic symbol spelling.
Not a duplicate of the earlier comment: that comment asked for the symbol/function unwind through nonoptional, which a0a0252 implemented; this finding is the implemented helper being narrower than the main walk it mirrors.
| // Never leave a floating rejection: an unhandled one crashes the process. | ||
| result.catch(() => {}); | ||
| return false; | ||
| } | ||
| return result.issues === undefined; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Whether the field's def chain carries a node that makes a missing key tolerable by | ||
| * construction — `default`/`prefault` (the default fills), a static `catch` (any | ||
| * input, including `undefined`, is replaced by the fallback), `optional`, an | ||
| * undefined-accepting leaf (`z.any()`/`z.unknown()`/`z.undefined()`/`z.void()`, or a | ||
| * literal whose values include `undefined`), or a Symbol-/function-typed leaf | ||
| * (JSON.stringify drops such keys from the payload entirely) — unwinding pipe sides, | ||
| * lazies, transparent wrappers, union members (ANY tolerant member suffices: zod | ||
| * tries members and the tolerant one succeeds), and intersections with EVERY-side | ||
| * semantics (`undefined` must parse through both sides). Deciding structurally matters | ||
| * because an async stage (`.refine(async ...)`, `.transform(async ...)`) pushes the | ||
| * validate-probe to a Promise. All checks err loosen-only: a false positive merely | ||
| * drops a field from the advertised `required`, which can never make a validating | ||
| * client reject a shipped payload. `ancestors` tracks the current traversal path so | ||
| * recursive lazies stay bounded. | ||
| */ |
There was a problem hiding this comment.
🟡 Stale JSDoc after the deferral fix: hasStructuralMissingKeyTolerance's header still lists 'default/prefault (the default fills)' as tolerance-by-construction and describes intersections as tolerant whenever 'undefined must parse through both sides', while fieldAcceptsMissingKey's header says a missing key passes 'true for .default()/.prefault()' — all three claims are contradicted by the branches in the same file (the prefault branch returns false, the intersection branch requires intersectionSidesFillDisjointObjects, and z.number().min(1).prefault(0) stays required per this PR's own pinned test). Reword the two headers to match the branch comments, e.g. 'default (the default fills; .prefault re-validates and defers to the probe)' and 'intersections only for the provably-mergeable disjoint-object-defaults shape'.
Extended reasoning...
The three stale claims
The a0a0252 deferral fix rewrote the prefault, intersection, and promise branches of hasStructuralMissingKeyTolerance and synced the branch comments — but not the two function-header JSDocs above them (packages/core-internal/src/util/standardSchema.ts around lines 875–900):
-
hasStructuralMissingKeyTolerance's header still opens with: tolerance by construction includes "default/prefault(the default fills)". The prefault branch ~40 lines below in the same function now returnsfalsewith the comment "UNLIKE.default(),.prefault(v)feeds v THROUGH the inner schema … Filling-then-revalidating is not filling: claim nothing and let the probe decide." Header and branch directly contradict each other. -
The same header still describes "intersections with EVERY-side semantics (
undefinedmust parse through both sides)" with no mention of the mergeability requirement the branch now enforces viaintersectionSidesFillDisjointObjects: both sides parsingundefinedis no longer sufficient — the branch comment itself says the two filled results "must MERGE", and only the disjoint-key plain-object-defaults shape is claimed structurally (everything else defers to the probe). -
fieldAcceptsMissingKey's header says a missing key passes validation "true for.default()/.prefault()" — false as a blanket claim:z.number().min(1).prefault(0)rejects a missing key, and this PR's own test ('prefault, unmergeable intersections, and promise fields stay advertised required') pins that field staying inrequired.
Step-by-step proof
Take z.object({ p: z.number().min(1).prefault(0), name: z.string() }):
- A maintainer reads either header and concludes
pis structurally missing-key-tolerant ('the default fills' / 'true for .default()/.prefault()'). - In reality
safeParse({name: 'n'})fails — the prefault value 0 is fed through and rejected bymin(1)— so every legitimately shipped payload carriesp. - On HEAD,
standardSchemaToJsonSchema(schema, 'output').requiredis['p', 'name']: the prefault branch returnsfalse, thevalidate(undefined)probe returns issues, and the field stays required — exactly what the pinned test asserts, and exactly the opposite of what both headers claim.
Why it matters
The misleading direction is precisely the false-tolerance bug the a0a0252 fix removed: a maintainer trusting the headers would conclude prefault fields and any both-sides-tolerant intersection are structurally droppable and could 'restore' the early-return, reintroducing the dropped-required/spurious-loosened bug. This matches the repo's REVIEW.md Documentation & Changesets recurring catch ('prose that promises behavior the code no longer ships … contradicts stated intent') and is the same missed-prose-sync class as the previously-fixed stale test title: the branch comments were synced in a0a0252, the function headers were not.
Why nothing else covers it
Not a duplicate of the earlier prefault/intersection/promise code-bug comments — those reported the code bugs, which a0a0252 fixed; this reports the prose the fix left stale. The earlier stale-prose finding covered a test title and the isProvablyObjectShapedRoot comment, different sites.
Fix
Reword the two headers to match the branches: e.g. in hasStructuralMissingKeyTolerance, 'default (the default fills; .prefault re-validates and defers to the probe)' and 'intersections only for the provably-mergeable disjoint-object-defaults shape (everything else defers to the probe)'; in fieldAcceptsMissingKey, drop .prefault() from the true-for list (or note it is probe-decided). Documentation-only — nothing breaks at runtime.
…egistry-shape carve-out inside id resources; serialization walk parity - The structural tolerance walk is checks-aware: a .refine()/.check() attaches to the SAME node and re-validates the filled/accepted value (.default(0).refine(v => v >= 1) rejects a missing key), so NO acceptance-based structural claim is sound on a checks-carrying node - it defers to the validate(undefined) probe (sync verdicts correct both ways; async checks conservatively stay required, folded into the async residual-gaps bullet). Symbol/function serialization tolerance is exempt (a value that passes its checks still never reaches the wire). Four pinned async spellings updated: async stages move DOWNSTREAM of the check-free tolerant node (bare-transform pipes, still structurally dropped) and the checks-on-node spellings pin the conservative stay-required posture. - plainObjectDefaultFill mirrors zod's isPlainObject gate: Dates/Maps/class instances have zero own enumerable keys (vacuously disjoint) yet zod merges them only when equal - non-plain fills defer to the probe (correct both directions per the same-timestamp Date control). - The id gates exempt #/$defs/<name> root-base pointers INSIDE id resources again: zod itself emits them there (z.lazy self-refs, cross-registered schemas), they resolve through the document root, and keeping the id made every SDK-client callTool fail at Ajv compile while disabling the whole loosen family. Bare '#' inside an id resource stays hand-authored (the round-34 fix's actual target). - hasSerializationDroppedLeaf mirrors the main walk: ANY-member union recursion and lazy-getter following (try/catch + cycle bound), so union- and lazy-nested symbols keep their wire-drop tolerance through nonoptional. - Synced the stale hasStructuralMissingKeyTolerance/fieldAcceptsMissingKey headers (prefault probe-decided; intersections only the provably-mergeable disjoint-object-defaults shape; checks-aware claims). Co-Authored-By: Claude <noreply@anthropic.com>
_Requested by Felix Weinberger
Before / After
Before: a single
z.date()(or any type zod can't represent in JSON Schema, e.g.z.bigint()) in any registered tool's schema made the entiretools/listrequest fail withMCP error -32603: Date cannot be represented in JSON Schema— every tool on the server disappeared. Separately, output schemas were advertised with constraints the server never enforces on the rawstructuredContentit ships:.default()-carrying fields were listed asrequiredand plainz.object()outputs carriedadditionalProperties: false, so validating clients (including the SDK's ownClient.callTool) rejected perfectly legitimate tool results with errors likedata must have required property 'counted', data must NOT have additional properties.After:
tools/listsucceeds —z.date()is advertised as{"type": "string", "format": "date-time"}(the shapeJSON.stringifyactually puts on the wire for aDate, matching the zod v3 converter), and other unrepresentable types degrade to an unconstrained schema instead of throwing. Advertised output schemas now describe the raw payload the server actually ships: defaulted fields are optional, andadditionalProperties: falseis only kept where zod really enforces it (z.strictObject()), so validstructuredContentpasses client-side validation.How
The conversion in
packages/core-internal/src/util/standardSchema.ts(standardSchemaToJsonSchema) called zod's converter with hardcoded options:~standard.jsonSchema[io]({ target })on the primary path andz.toJSONSchema(schema, { target, io })on the zod 4.0–4.1 fallback. This PR adds azodConversionOptions(io)helper —unrepresentable: 'any'plus anoverridehook that rewritesZodDatenodes tostring/date-timeand, forio: 'output'object nodes, drops non-strictadditionalProperties: falseand removes defaulted properties fromrequired. It is passed vialibraryOptionson the Standard JSON Schema path (gated tovendor === 'zod', so other Standard Schema vendors are untouched) and spread into the fallback call. This mirrors the option semantics of the v1.x fix in #2467, adapted to the v2 layout's standard-schema conversion path.Tests: schema-level regression tests in
packages/core-internal/test/util/standardSchema.test.ts(plus a fallback-path test), and end-to-end server tests inpackages/server/test/server/toolSchemaWireShape.test.tsthat verifytools/listsucceeds with az.date()tool registered and that shippedstructuredContent(omitting a defaulted field, carrying an extra key) validates against the advertisedoutputSchemawith the SDK's own Ajv validator — all of which fail without the fix. A patch changeset forcore-internalandserveris included.Fixes #2464
Generated by Claude Code